Skip to content

fix: two CRM end-to-end runtime gaps (delete cascade + dead convert route)#1984

Merged
os-zhuang merged 1 commit into
mainfrom
fix/crm-e2e-gaps
Jun 16, 2026
Merged

fix: two CRM end-to-end runtime gaps (delete cascade + dead convert route)#1984
os-zhuang merged 1 commit into
mainfrom
fix/crm-e2e-gaps

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Found by driving the app-crm example as a real user end-to-end (boot → sign-up/session → CRUD → record clone → aggregation → domain actions → RBAC → console UI). The core path is solid (auth, CRUD, clone, aggregation, audit stamping, metadata, console all work); these two are the runtime gaps that surfaced.

1. Delete of a parent with a required-FK child failed with a misleading error (data layer)

Deleting a crm_account that has crm_opportunity children returned:

400 {"error":"account is required","code":"VALIDATION_FAILED","fields":[{"field":"account",...}]}

…naming account, a field that isn't even on crm_account. Root cause: cascadeDeleteRelations defaults a lookup FK to set_null; for the required crm_opportunity.account FK it issued an UPDATE clearing the column, which the child's required-field validator rejected. A required FK can't be nulled — that's a contradiction, not the author's intent.

Fix: a defaulted set_null on a required FK now escalates to restrict (SQL's standard for a NOT NULL FK):

409 {"error":"Cannot delete crm_account (…): 4 dependent crm_opportunity record(s) reference it via account
     (account is required, so it cannot be cleared). Delete or reassign them first, or set
     deleteBehavior:'cascade' on crm_opportunity.account.",
     "code":"DELETE_RESTRICTED","dependentObject":"crm_opportunity","dependentCount":4}

Explicit cascade/restrict and optional (nullable) lookups are unchanged. New engine-cascade-delete.test.ts (4 cases: required→restrict, no-dependents→ok, optional→set_null, explicit cascade→removed).

2. Removed the hardcoded POST /data/lead/:id/convert endpoint + convertLead (false surface)

The route and convertLead baked bare object names (lead/account/contact/opportunity) and a fixed Salesforce field mapping into the framework runtime. So it's unreachable by any real app: /data/crm_lead/:id/convert → 404, and the literal lead object doesn't exist. The CRM models conversion correctly as a flow (crm_convert_lead_wizard screen flow) — a CRM-specific workflow doesn't belong hardcoded in the framework. Untested, undocumented, no consumers. Removed (narrow-and-true).

Verification

  • Live CRM run (rebuilt packages): delete-with-children → 409 clear message; childless delete → 200; record clone (same data-action group) unaffected → 201; convert route gone → 404.
  • @objectstack/objectql 637 (+4 cascade tests), @objectstack/rest 121 green.

Not fixed here — needs a product call (Gap 3)

A third finding is a deliberate posture, not a bug, so I did not flip it unilaterally: a freshly-authenticated user who resolves to zero permission sets has object-permission enforcement skipped (security-plugin.ts:325 if (permissionSets.length > 0)), i.e. fail-open when unconfigured. This lets apps with no security metadata "just work," but means once an app defines permission sets without assigning them to every user, unassigned users get full read/create instead of none. Flipping to default-deny would break every app that doesn't assign permission sets to all users (showcase, todo, the CRM itself). Tracking separately for a design decision (default-deny-when-object-has-any-permission-set, a default authenticated permission set, or an explicit secure-by-default flag).

🤖 Generated with Claude Code

…oute)

Found by driving the app-crm example as a real user (auth → CRUD → actions).

1. Delete of a parent with a REQUIRED-FK child failed with a misleading
   validation error. cascadeDeleteRelations defaulted a lookup FK to
   set_null; for a required FK that UPDATE-cleared the column, which the
   child validator rejected with `400 "<field> is required"` — naming a
   field not even on the object being deleted (delete crm_account w/
   opportunities → "account is required"). A required FK can't be nulled,
   so a *defaulted* set_null now escalates to restrict: refuse with a clear
   `409 DELETE_RESTRICTED` { dependentObject, dependentCount } and an
   actionable message. Explicit cascade/restrict and nullable lookups are
   untouched. New engine-cascade-delete.test.ts (4 cases).

2. Removed the hardcoded `POST /data/lead/:id/convert` route + convertLead
   protocol method. It baked bare object names (lead/account/contact/
   opportunity) + a fixed field mapping into the framework runtime, so it
   was unreachable for any namespaced app and unused by the CRM (which
   models conversion as a `crm_convert_lead_wizard` screen flow). False
   surface — removed. Untested/undocumented; no consumers.

Verified live against a running CRM: delete-with-children → 409 w/ clear
message; childless delete → 200; clone (same data-action group) unaffected;
convert route gone. objectql 637, rest 121 green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jun 16, 2026 4:27pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/rest.

15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/core/services.mdx (via @objectstack/objectql)
  • content/docs/concepts/implementation-status.mdx (via @objectstack/objectql, @objectstack/rest)
  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/concepts/packages.mdx (via @objectstack/objectql, @objectstack/rest)
  • content/docs/guides/api-reference.mdx (via @objectstack/rest)
  • content/docs/guides/authentication.mdx (via @objectstack/objectql)
  • content/docs/guides/deployment-vercel.mdx (via @objectstack/objectql)
  • content/docs/guides/formula.mdx (via packages/objectql)
  • content/docs/guides/kernel-services.mdx (via @objectstack/objectql)
  • content/docs/guides/objectql-migration.mdx (via @objectstack/objectql)
  • content/docs/guides/packages.mdx (via @objectstack/objectql, @objectstack/rest)
  • content/docs/guides/plugins.mdx (via @objectstack/objectql, @objectstack/rest)
  • content/docs/protocol/objectos/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)
  • content/docs/releases/v9.mdx (via @objectstack/objectql)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang merged commit 44c5348 into main Jun 16, 2026
13 of 14 checks passed
@os-zhuang
os-zhuang deleted the fix/crm-e2e-gaps branch June 16, 2026 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant